feat(contexts): data model for grid pages - #3131

Do we need an icon and a description on widgets now?

Yes, I'd like to keep them. Pages and menu items have an optional icon and description too, so all three use the same icon picker and the same fields. The frontend shows them in the widget header. Both are optional and stay NULL when nobody sets them.

i thought we said we're using JSON?

We did, sorry. The column is Types::JSON now. Nextcloud maps it per database: json on PostgreSQL, JSON on MySQL, LONGTEXT on MariaDB and text on SQLite. I ran the tests on PostgreSQL and SQLite; nothing compares the stored text, so MySQL normalising the JSON doesn't matter. The Widget entity maps the field as Types::JSON too and reads and writes it as an array. That part is in #3133, where the settings type moves into ResponseDefinitions anyway.

The page type from the import file isn't checked. Hwo are we sure we're only accepting startpage and grid? Can someone create two start pages, for example?

Good catch. An import now only takes startpage and grid pages, and at most one start page per application. Any other page is skipped with a warning in the log, like other invalid import data. There's a test with a second start page, an unknown type, a page without a type and one that isn't an object.

Should this be ?string?

Yes. The column allows NULL for the moment between inserting a page and giving it page_<id>, so the getter and setter are ?string now, like the other nullable fields.

In this file and other test file, it seems like we have more test code than feature code. Most of the extra comes from the same case being tested in two or three places, or from tests that check mocks or config instead of behaviour. We should cut out some tests or merge. Having too many tests which are not useful slows down CI and adds clutter

Agreed, there was too much. I went through the tests of #3131, #3132 and #3133: one test per behaviour, cases that were tested in two or three places kept in one, near duplicates merged into data providers, and tests that only checked mocks, config or getters and setters removed. For this PR that's 36 test methods down to 22, and 817 test lines down to 691.

#3132

Same concern for technical names as noted about menu entries in #3100 (comment)

It can store Json, but it has to be TEXT. JSON is also deprecated as of NC 33: https://github.com/nextcloud/server/blob/b117761f029381403cdba2e647c1f43fb6bd73fc/lib/public/DB/Types.php#L177-L180

In theory, we can run into a memory issue here, hence better to deal with one result row at a time. Now, I am not expecting huge amounts of data there, should be fine hence.

hm where does the disparity between technical_name and technicalName come from? Same about the page type below.

Imported pages should be referenced by their exported id, not by technical name. The menu export will refer to pages by technical name. But importedPageNames() renames pages on import:

• it drops page_<n> names and duplicates;
• dashboard can move to another page

So looking up a page by name after import can return the wrong page or none at all. importContext() already builds $pageMap keyed by the exported page id, the same way $nodeRelMap works. Pages are always created new on import, so the exported id is enough to find them. Using it also handles the concern that technical names are mutable aliases and shouldn't serve as identifiers.

should be within a transaction

#2476 bumps to the same 2.5.0-dev.0 and ships its own migration (Version2500Date20261031000000). The two branches merge without a conflict on this line, so whichever lands second leaves the version unchanged, and Nextcloud only upgrades an app when the info.xml version differs from installed_version (AppManager::isUpgradeRequired()).

On an instance that already runs the first one (dev or nightly setups tracking main), Version2500Date20261022000000 then stays pending until the next bump: tables_contexts_page has no technical_name column, and ContextService::insertPage() now always writes it, so creating any application fails with an unknown-column error. A dev instance that ran #2476 is in exactly that state (installed_version 2.5.0-dev.0, page table without technical_name). In the other merge order, #2476's archive tables are the ones missing.

Whichever of the two lands second needs to bump past what main has at that point, and #3135 then one higher.

One more thing on the ordering, checked on a dev instance running #2476: it has already applied Version2500Date20261031000000 and Version2500Date20261101000000, while this PR's migration is Version2500Date20261022000000, so it sorts before both. Whichever lands second, please rename it to the then-current target version and a current date, as AGENTS.md asks for long-lived PRs, so it sorts after everything already applied.

Grid pages are importable, but this loop never gives them a title. From #3132 on, the import keeps the title NULL whenever the exported one is missing, blank or too long, while the API (PageService::validateTitle() in #3133) requires 1 to 200 characters for a grid page.

In the page bar of the UI stack (ContextPageBar.vue), pageTitle() returns '' for such a page, so its role="tab" button contains only the aria-hidden icon and an empty label. Screen readers announce an unnamed tab, and sighted users see an unlabeled icon (WCAG 4.1.2 Name, Role, Value).

Giving an imported grid page without a usable title a fallback title (its technical name, for example), or skipping it like other invalid page entries, keeps the stored data within what the API allows. A fallback label in pageTitle() would cover the UI side.

The source is 0..1 per widget (primary key on widget_id), so would a nullable, indexed node_rel_id column on tables_contexts_widget do the same job? Both reasons from #3100 hold for a column too: it's still a real column the server can check and clean up, and the own-application check (filterNodeRelIdsOfPageContext() in #3132) works the same way. The menu table in #3135 already stores its node_rel_id that way, as a nullable, indexed column on the row itself.

With the separate table, every read needs a second query or a join, and syncPage() in #3132 calls setSource() for every listed widget on every save: an unchanged page with N widgets still runs N DELETEs plus one INSERT per data widget, where an unchanged column would be skipped together with the widget's own UPDATE. A column would also make WidgetSource, WidgetSourceMapper and the delete-then-insert in setSource() unnecessary.

This keeps at most one start page, but nothing guarantees one. An archive whose application has only grid pages (or no pages) imports as an application without a start page. Adding a table to it later goes through addNodeRelToPage(), which finds no start page and passes null to PageContentMapper::findByPageAndNodeRelation(int $pageId, ...), so the request ends in a TypeError (500) after the node relation row is already written, because update() isn't atomic.

Genuine exports always contain a start page, so this needs an edited or damaged archive, and it behaved the same before this PR. Since the import validates pages here now, it may be worth closing. A fallback would have to cover a missing pages key, can't assume dashboard is free (a grid page may keep that name), and should put the imported node relations on the new start page.

AGENTS.md: "IN clauses must be chunked to at most 1 000 items for Oracle compatibility. Use a named constant (DB_CHUNK_SIZE = 1_000)", as ContextMapper, ViewMapper, TableMapper, Row2Mapper and RowSleeveMapper do. None of the five new IN queries chunk: findByPageIds() and deleteByIds() here, and findNodeRelIdsByWidgetIds(), deleteByWidgetIds() and deleteByNodeRelIds() in WidgetSourceMapper.

In the stack, deleting or exporting an application passes every widget id of that application. With 100 widgets per page and no page limit, more than 1000 ids are reachable, and the server then logs "More than 1000 expressions in a list are not allowed on Oracle." at error level on every database. When chunking findNodeRelIdsByWidgetIds(), write each chunk into the same map: array_merge(...) would renumber the widget-id keys.

Related, in #3132: WidgetService::deleteByPageIds() calls findIdsByPageId() once per page with array_merge inside the loop, which AGENTS.md also rules out. findByPageIds() already covers it in one query.

Nothing in the stack calls this outside the tests. The API and export shape is built by hand in WidgetService::findByPageIds() (#3132) and differs from this one: it has source and no pageId, matching TablesWidget in #3133. Dropping JsonSerializable and asserting getSettingsObject() directly in WidgetTest and WidgetMapperTest would leave one widget shape.

QBMapper::insert() doesn't reset the updated fields, so the update() below re-sends every field set before the insert (context_id, page_type, ...) together with technical_name. Resetting first narrows it to SET technical_name = ?, the same way ContextService::create() resets after its insert.

Nothing in this PR uses this class; only bootstrap.php requires it, and its first user is PageServiceTest in #3133. Moving the file and the require_once there keeps this PR to the data model. A createMock(Exception::class) with getReason() stubbed would also work, since getReason() isn't final.

Now that the rule lives in IconHelper, this method only forwards to it. Nothing overrides or mocks it, and the later PRs call IconHelper::isValid() directly, so the two callers (lines 114 and 155) could do the same and the method could go.

This reaches the private assertTechnicalNameValid() through reflection on an instance built without its constructor, so renaming or inlining the helper breaks the test without any change in behaviour. ViewServiceAuditLogTest already builds a ViewService from mocks; calling create(..., technicalName: ...) there would keep both message assertions, including the length message now built from MAX_LENGTH, on the public path.